[07-C] [Project Solar / Phase 1 / Engineering Follow-ups] Improve validation for $modes
#3397
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR enhances validation for design token $modes entries in the build script to catch configuration errors early. It adds two levels of validation: a warning for mismatched default mode values and an error that halts execution when expected mode keys are missing.
Key changes:
- Added chalk dependency for improved console output formatting
- Implemented soft validation warning when
$modes.defaultdiffers from$value - Converted TODO comment into hard validation that throws an error for missing mode keys
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/tokens/scripts/build.ts | Added validation logic for $modes with chalk-formatted console messages |
| packages/tokens/package.json | Added chalk dependency for colored console output |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| function replaceModes(slice: DesignToken) { | ||
| if (slice.$modes) { | ||
| if (mode in slice.$modes) { | ||
| // extra validation to catch instances where the `default` mode value is different from the `$value` |
There was a problem hiding this comment.
Using file: packages/tokens/src/products/shared/typography.json
On line 158, I tested changing the font-weight regular $value to "900" so that it no longer matched the $modes default value of "400" then ran pnpm build.
I saw a warning as expected, but it was repeated twice:
Processing mode "default"...
⚠️ WARNING - Found themed 'default' token with value different than '$value' (`400` instead of the expected `900`) - File: src/products/shared/typography.json
⚠️ WARNING - Found themed 'default' token with value different than '$value' (`400` instead of the expected `900`) - File: src/products/shared/typography.json
I tested again, changing another value and was also seeing a duplicated warning.
| // TODO! decide if we want to throw here (and test if it works, by removing a value from one of the test files) - see: https://hashicorp.atlassian.net/browse/HDS-5668 | ||
| console.error(`❌ ERROR - Found themed token without '${mode}' value:`, JSON.stringify(slice, null, 2)); | ||
| // we want to interrupt the execution of the script if one of the expected modes is missing | ||
| throw new Error(`❌ ${chalk.red.bold('ERROR')} - Found themed token without '${mode}' value - ${JSON.stringify(slice, null, 2)}`); |
There was a problem hiding this comment.
Using file: packages/tokens/src/products/shared/typography.json
I tried deleting one of the $modes entries and ran pnpm build and received an error as expected.
Processing mode "cds-g0"...
/Users/kristinbradley/dev/design-system/packages/tokens/scripts/build.ts:53
throw new Error(`❌ ${chalk.red.bold('ERROR')} - Found themed token without '${mode}' value - ${JSON.stringify(slice, null, 2)}`);
^
Error: ❌ ERROR - Found themed token without 'cds-g0' value - {
"$type": "font-family",
"$value": "{typography.font-stack.monospace.code.combined}",
"$modes": {
"default": "{typography.font-stack.monospace.code.combined}",
"cds-g10": "{carbon.typography.font-family.mono}",
"cds-g90": "{carbon.typography.font-family.mono}",
"cds-g100": "{carbon.typography.font-family.mono}"
},
KristinLBradley
left a comment
KristinLBradley
left a comment
There was a problem hiding this comment.
Looks good other than the duplicated warning messages I was seeing when I tested.
3 participants
📌 Summary
Small PR that adds some basic validation to the design tokens
buildscript, to make sure the$modesentry have the expected format and values.🛠️ Detailed description
In this PR I have:
chalkas devDependency (used to highlight some text in the console messages)replace-value-for-mode-${mode}preprocessing function:$modes.defaultvalue is different from the standard$value$modeskeys is missingNote: later we may decide to make also the first validation a blocker, for now we want to test how this "soft" check works for the developers adding tokens for the migration of the HDS components to Carbon
🔗 External links
Jira ticket: https://hashicorp.atlassian.net/browse/HDS-5668
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.
📋 PCI review checklist
Examples of changes to controls include access controls, encryption, logging, etc.
Examples include changes to operating systems, ports, protocols, services, cryptography-related components, PII processing code, etc.